-
-
Notifications
You must be signed in to change notification settings - Fork 170
CYF - London | Anna Fedyna | Module-Structuring-and-Testing-Data | Week 3 #91
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This reverts commit 1f482ea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have left you some feedback.
Sprint-1/errors/4.js
Outdated
const hourClockTime12 = "20:53"; | ||
const hourClockTime24 = "08:53"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assessment of problem is correct, but there seem further issues with the original variable declarations. I suggest taking a closer at the values of the variables in relation to the variable names to find the issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, thank you, the variable names and values were mismatched.
It should be :
const hourClockTime24 = "20:53"; const hourClockTime12 = "08:53";
Sprint-1/exercises/count.js
Outdated
@@ -4,3 +4,5 @@ count = count + 1; | |||
|
|||
// Line 1 is a variable declaration, creating the count variable with an initial value of 0 | |||
// Describe what line 3 is doing, in particular focus on what = is doing | |||
|
|||
/* the = operator here is taking the result of the expression on the right and assigning it to variable count */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel it would help to provide slightly more explanation as to what's happening here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course, I will add here a bit more detailed explanation about what's happening with the = operator in JavaScript. So, in line 3, the = operator is used to assign the value on the right-hand side to the variable on the left-hand side. We have count = count + 1 . First, the value of count (which is initially 0) is accessed. Then, 1 is added to it, resulting in 1.This value (1) is assigned back to the variable count. The = operator here doesn't mean "equal" . Instead, it acts as an assignment operator which updates the variable with the new value.
Sprint-1/exercises/decimal.js
Outdated
@@ -7,3 +7,11 @@ const num = 56.5678; | |||
// Create a variable called roundedNum and assign to it an expression that evaluates to 57 ( num rounded to the nearest whole number ) | |||
|
|||
// Log your variables to the console to check your answers | |||
|
|||
const wholeNumberPart = Math.floor(num); | |||
const decimalPart = num - Math.floor(num); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not give the expected result, as it has too many decimal places. Perhaps there is a way to limit them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can use toFixed() to limit decimal places
Sprint-1/exercises/decimal.js
Outdated
|
||
|
||
|
||
console.log(roundedNum); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You were asked to log all the variables, not just one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for pointing out!
Sprint-1/exercises/paths.js
Outdated
// Create a variable to store the ext part of the variable | ||
const extPart = base.slice(base.lastIndexOf('.')+1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look at the diagram above and see if it matches up exactly with your solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, after reviewing the diagram and running the code it matches, please let me know if solution not aligns correctly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please take note of all the characters under the 'base' column
Sprint-1/exercises/random.js
Outdated
/* | ||
(maximum - minimum + 1) = 100 | ||
Math.random() - generates a random floating-point number between (0, 1) | ||
Math.floor() - rounds down to the nearest whole number |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the necessity for rounding down rather than rounding up?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So that num is not bigger than 100
Sprint-1/exercises/random.js
Outdated
@@ -7,3 +7,15 @@ const num = Math.floor(Math.random() * (maximum - minimum + 1)) + minimum; | |||
// Try breaking down the expression and using documentation to explain what it means | |||
// It will help to think about the order in which expressions are evaluated | |||
// Try logging the value of num and running the program several times to build an idea of what the program is doing | |||
|
|||
/* | |||
(maximum - minimum + 1) = 100 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the purpose behind the multiplication?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The purpose is to scale the random value
I suppose this PR is for sprint-3 (since it says week 3)? Apparently, this is a common mistake. See if can fix the issue from the suggestions found on Slack: |
Thank you very much for pointing out ! I rebased two branches. |
Learners, PR Template
Self checklist
Changelist
Briefly explain your PR.
Questions
Ask any questions you have for your reviewer.
I’d appreciate it if you could review whether my input checks are efficient, especially when checking if a value is a number in get-angle-type.test.js !